infinite while loop fix write() and clientWrite() #2
Conversation
Reimplemented return logic in BearSSLClient::write() and BearSSLClient::clientWrite() in order to avoid infinite loop in https://github.com/Rocketct/ArduinoBearSSL/blob/master/src/bearssl/ssl_io.c#L279 when networking error happend in low level client(GSMClient)
There was a problem hiding this comment.
@Rocketct do you have a consistent way to reproduce this issue?
I think we only need changes to BearSSLClient::write to fix this.
src/BearSSLClient.cpp
Outdated
| @@ -289,5 +290,5 @@ int BearSSLClient::clientWrite(void *ctx, const unsigned char *buf, size_t len) | |||
|
|
|||
| int w = c->write(buf, len); | |||
There was a problem hiding this comment.
Client::write returns a size_t type which is unsigned, I think it's clearer if w is changed to size_t instead of int then we don't need the change below.
There was a problem hiding this comment.
The point is that this return allow to break the while in the point of the following link, https://github.com/Rocketct/ArduinoBearSSL/blob/master/src/bearssl/ssl_io.c#L279, in case of GSMClient faillure.
If instead of a -1 it return size=0 could be possible that len in https://github.com/Rocketct/ArduinoBearSSL/blob/master/src/bearssl/ssl_io.c#L283 is never decremented.
src/BearSSLClient.cpp
Outdated
| @@ -41,15 +41,16 @@ int BearSSLClient::connect(const char* host, uint16_t port) | |||
|
|
|||
| size_t BearSSLClient::write(uint8_t b) | |||
| { | |||
|
|
|||
| return write(&b, sizeof(b)); | |||
| } | |||
|
|
|||
| size_t BearSSLClient::write(const uint8_t *buf, size_t size) | |||
| { | |||
| br_sslio_write_all(&_ioc, buf, size); | |||
There was a problem hiding this comment.
Should we also check the return value of br_sslio_write_all ?
|
yes the following sketch reproduce the problem, is a simplified version without of the one that i use: #include <MQTTClient.h>
#include "MKRNBIoT.h"
#include "arduino_secrets.h"
#include <ArduinoBearSSL.h>
#define GPRS_APN "22210"
#define PINNUMBER ""
void connect();
GSMClient net;
GPRS gprs;
GSM gsmAccess(true);
GSMScanner scannerNetworks;
BearSSLClient SSLClient(net);
MQTTClient mqttclient;
const int publishInterval = 5000;
unsigned long lastMillis = 0;
float lastMeasure;
unsigned long getTime() {
return gsmAccess.getTime();
}
void setup() //This code is executed once
{
//Peripheral Initialization
Serial.begin(115200);
connect();
}
void connect() {
Serial.print("Connecting to GSM networks");
boolean connected = false;
while (!connected) {
if ((gsmAccess.begin(PINNUMBER) == GSM_READY) &&
(gprs.attachNBIoT(GPRS_APN) == GPRS_READY)) {
connected = true;
} else {
Serial.println("Not connected");
delay(1000);
}
}
Serial.println("\nConnected!\n");
ArduinoBearSSL.onGetTime(getTime);
Serial.println("Connecting to MQTT");
mqttclient.begin(MQTT_URL, 8883, SSLClient);
while (!mqttclient.connect(MQTT_ID, MQTT_USR, MQTT_PWD)) {
Serial.println("Not connected");
delay(1000);
}
Serial.println("\nConnected!\n");
}
unsigned long int i = 0;
void loop() //This code is looped forever
{
if (!mqttclient.connected()) {
connect();
}
mqttclient.loop();
// publish a message roughly every second.
if (millis() - lastMillis > publishInterval) {
lastMillis = millis();
publishLevel(i%10);
}
}
bool publishLevel(int level) {
char stringToPublish[100];
snprintf(stringToPublish, 100, "%s%d%s",
"{\"data\": {\"level\":\"",
level,
"\"}}");
return publish(String (stringToPublish));
}
bool publish(String (stringToPublish)) {
if (mqttclient.publish(MQTT_MEASURES, stringToPublish)) {
Serial.print("Published: ");
Serial.println(stringToPublish);
return true;
}
else {
Serial.println("Failed");
mqttclient.disconnect();
return false;
}
}
|
Reimplemented logic of BearSSLClient::write(const uint8_t *buf, size_t size) function
Reimplemented return logic in BearSSLClient::write() and BearSSLClient::clientWrite() in order to avoid infinite loop in https://github.com/Rocketct/ArduinoBearSSL/blob/master/src/bearssl/ssl_io.c#L279 when networking error happend in low level client(GSMClient)